Skip to content

refactoring root.array && fix error#22327

Draft
gorsing wants to merge 1 commit intodlang:masterfrom
gorsing:fixroot_array
Draft

refactoring root.array && fix error#22327
gorsing wants to merge 1 commit intodlang:masterfrom
gorsing:fixroot_array

Conversation

@gorsing
Copy link
Contributor

@gorsing gorsing commented Dec 30, 2025

Refactor dmd.root.Array: Fix debug stomping logic, optimize toString, and improve ergonomics

Description:

This MR introduces several improvements to the dmd.root.Array struct, focusing on memory safety in debug modes, better ergonomics for slice usage, and optimized string conversion.

Key Changes:

1. Fix Memory Stomping Logic (Debug Mode)

  • Fixed memset size calculation: In the previous implementation, the destructor and reserve function (under debug (stomp)) passed data.length (element count) to memset, instead of data.length * T.sizeof (byte count). This meant that for types larger than 1 byte, only a fraction of the memory was being "stomped" (overwritten with 0xFF), potentially hiding use-after-free bugs.
  • Protected Small Array Storage: Added checks (data.ptr != &smallarray[0]) in the reserve debug logic to ensure we do not attempt to memset or free the internal inline storage (Small Array Optimization) during resizing or destruction.

2. Improve Ergonomics

  • Added alias opSlice this: The Array struct now includes alias opSlice this;.
  • Benefit: This allows Array!T to implicitly convert to T[]. It can now be passed directly to functions expecting a slice or a standard D range (e.g., foreach loops, std.algorithm) without explicitly calling .data or [].

**3. Refactor toString**

  • Optimization: Added an early exit for empty arrays to avoid unnecessary allocation.
  • Cleanup: Rewrote the string construction logic. The new implementation uses a cleaner loop for separator handling (, ) and correctly calculates buffer sizes, replacing the previous complex pointer arithmetic.

4. Minor Cleanups

  • Simplified the variadic constructor implementation to be more direct.

@dlang-bot
Copy link
Contributor

dlang-bot commented Dec 30, 2025

Thanks for your pull request and interest in making D better, @gorsing! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#22327"

@gorsing gorsing mentioned this pull request Dec 30, 2025
Draft
@gorsing gorsing force-pushed the fixroot_array branch 2 times, most recently from 39bf153 to 9ce3a33 Compare December 30, 2025 22:36
@dkorpel
Copy link
Contributor

dkorpel commented Jan 1, 2026

Happy new year!

I have some comments about each of the key changes, but please keep each PR focused on 1 issue. It's fine to bundle a handful of trivial changes or include a typo fix here and there of course, but especially keep the 3 categories of PRs separate:

  • Bug fixes
  • Optimization
  • Refactoring

Mixing such changes makes it harder to review and test, but especially in the past it made bisecting regressions harder, when for example a crucial 1-line change was buried in a diff of 100 lines of reformatting.

@gorsing gorsing marked this pull request as draft January 2, 2026 10:04
@gorsing
Copy link
Contributor Author

gorsing commented Jan 2, 2026 •
edited
Loading

Happy new year!

Hi @dkorpel
Thank you for the feedback. I agree that mixing these changes makes review and bisecting difficult.
I will close this PR and split the changes into many simple separate PRs as requested:

Bug fixes: Fixes for memset size in destructor and safety checks in reserve.
Refactoring: alias opSlice this and the toString implementation cleanup.
Optimization: implementation improvements for the array constructor.

I'll link the new PRs here once they are created."

@gorsing gorsing mentioned this pull request Jan 2, 2026
Merged
@gorsing gorsing mentioned this pull request Jan 6, 2026
Merged
@WalterBright
Copy link
Member

WalterBright commented Jan 10, 2026

@gorsing thank you for the excellent opening summary.

@gorsing
Copy link
Contributor Author

gorsing commented Jan 12, 2026

Thanks, @WalterBright I'm glad the summary was helpful. As discussed with @dkorpel, I've split this work into separate PRs (including #22344 and #22351) to make reviews easier. I'll go ahead and close this one now.

@gorsing
Copy link
Contributor Author

gorsing commented Jan 13, 2026

#22392

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gorsing @dlang-bot @dkorpel @WalterBright